Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove TOP_TABLES_BY_WASTED_SPACE_COUNT from UiConstants #1757

Merged
merged 2 commits into from
Jul 27, 2017

Conversation

europ
Copy link
Member

@europ europ commented Jul 26, 2017

Issue: #1661

We have removed TOP_TABLES_BY_WASTED_SPACE_COUNT constant from UiConstants. One occurrence of constant TOP_TABLES_BY_WASTED_SPACE_COUNT was replaced with its value in manageiq-ui-classic/app/helpers/ops_helper/textual_summary.rb.

@miq-bot
Copy link
Member

miq-bot commented Jul 26, 2017

This pull request is not mergeable. Please rebase and repush.

@europ europ force-pushed the remove-ui-constants-8 branch from 3ebdbe0 to a51f486 Compare July 26, 2017 09:53
@miq-bot
Copy link
Member

miq-bot commented Jul 26, 2017

This pull request is not mergeable. Please rebase and repush.

@europ europ force-pushed the remove-ui-constants-8 branch from a51f486 to 0bc7a40 Compare July 26, 2017 11:03
@@ -125,7 +125,7 @@ def textual_vmdb_tables_most_wasted_space
:title => _("Tables with Most Wasted Space"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@europ move the constant definition on the beginning of this method, instead of leaving a random number in the code

Copy link
Member

@martinpovolny martinpovolny Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romanblanco , I see your point. But this constant is really in just one place and we removed more constants like that. Maybe we could replace more of the "5" that serve similar purpose with a single constant in a follow up PR?

@miq-bot
Copy link
Member

miq-bot commented Jul 26, 2017

This pull request is not mergeable. Please rebase and repush.

@europ europ force-pushed the remove-ui-constants-8 branch from 0bc7a40 to 2506bf7 Compare July 26, 2017 22:08
@miq-bot
Copy link
Member

miq-bot commented Jul 26, 2017

Checked commits europ/manageiq-ui-classic@46f3df6~...2506bf7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@martinpovolny martinpovolny merged commit b8e4d86 into ManageIQ:master Jul 27, 2017
@martinpovolny martinpovolny self-assigned this Jul 27, 2017
@martinpovolny martinpovolny added this to the Sprint 66 Ending Aug 7, 2017 milestone Jul 27, 2017
@europ europ deleted the remove-ui-constants-8 branch August 7, 2017 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants